add ncpus_per_host option to LSFCluster#176
Conversation
willirath
left a comment
There was a problem hiding this comment.
There's a lot of re-formatted code that has nothing to do with the actual changes. This is a problem: git blame then won't be of any use in the affected lines, because it'll point to the commit that did the reformat rather than the one that implemented functionality.
I'd recommend reverting all but the changes that really change behaviour and, if necessary, deal with any code style issues separately.
|
@willirath the reformatting is done in separate commits, I don't understand what you mean. |
|
Sorry, I was not as verbose as I should have been: All these commits will be squashed to one commit representing this PR. Then, we'll have |
|
I'm am no expert in github, but I think it only happens if you actually squash. Since I forked directly from the current HEAD, you can actually do Rebase and merge. A merge commit (the default) will also work. If you prefer, I can cherry-pick the commits and turn off my editor's autoformatting. |
|
Hm, I think @willirath has got a point. It will be clearer to avoid reformatting for this issue. |
|
Ok, I'll redo it. Anyway I think I'll need to change my code depending on your answer on #172. Just for my personal knowledge: what is the point of using only "squash and commit"? |
The master branch evolving in steps that make sense from a functional point of view rather than reflecting the (often random) steps it took to develop and implement the solution. Taking this PR as an example, we'd have one commit telling the reader "LSFCluster now correctly starts all threads on the same node and here's all we did to achieve that." vs different commits that make it very hard to later understand what happened. |
|
Thanks anyway @louisabraham for all the work you've already done, and for sticking with this! |
|
I am the one who thanks both of you for actually taking more time to explain things to me than it would take you to fix the bug :) |
I'm glad you take it this way! Communicating criticism in a well-meaning and constructive manner is not trivial and it'd probably be easy to read my remarks in an offensive tone. |
fix #172